raft: fix flaky leader index in waitLeader function#130084
Conversation
nvb
added
backport-24.1.x
|
This change is |
pav-kv
left a comment
pav-kv
left a comment
There was a problem hiding this comment.
Thanks for picking this up.
|
Could you remove the TODO / NB: Back then I thought that this test was inherently flaky and the fix was incomplete. There was some analysis in etcd-io/raft#195 (comment) and other comments there. We can revisit if this flake comes up again after your PR. |
| func waitLeader(ns []*node) int { | ||
| var l map[pb.PeerID]struct{} | ||
| var lindex int | ||
| var lindex = -1 |
There was a problem hiding this comment.
Should this be reset to -1 on each iteration? Might be better to move the definition inside the loop then.
There was a problem hiding this comment.
Done. I also moved to clearing the map on each iteration, instead of re-allocating it.
I stressed this now locally 20k times with the fix applied (one with the |
Done. |
Fixes cockroachdb#127413. This commit bypasses the larger rebase in cockroachdb#122133 to pick up the test flake fix in etcd-io/raft#188. There was some discussion in etcd-io/raft#181 about alternatives for fixing this test. For now, we stick with a direct cherry-pick. Release note: None
nvb
force-pushed
the
nvanbenschoten/deflake127413
branch
from
c429d4e to
bd9edcb
Compare
I stressed again for 20k iterations and still see that it is stable. bors r=pav-kv |
|
Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches. Issue #127413: branch-release-24.1, branch-release-24.2. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from bd9edcb to blathers/backport-release-24.1-130084: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.1.x failed. See errors above. error creating merge commit from bd9edcb to blathers/backport-release-24.2-130084: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
3 participants
Fixes #127413.
This commit bypasses the larger rebase in #122133 to pick up the test flake fix in etcd-io/raft#188. There was some discussion in etcd-io/raft#181 about alternatives for fixing this test. For now, we stick with a direct cherry-pick.
Release note: None